Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[exporter/prometheusremotewrite] Fix WAL deadlock #37630

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ArthurSens
Copy link
Member

@ArthurSens ArthurSens commented Jan 31, 2025

I was taking a look over #20875 and hoping to finish it.
Fixes #19363
Fixes #24399
Fixes #15277


As mentioned in #24399 (comment), I used a library to help me understand how the deadlock was happening. (1st commit). It showed that persistToWal was trying to acquire the lock, while readPrompbFromWal held it forever.

I changed the strategy here and instead of using fs.Notify, and all that complicated logic around it, we're just using a pub/sub strategy between the writer and reader Go routines.

The reader go routine, once finding an empty WAL, will now release the lock immediately and wait for a notification from the writer. While previously it would hold the lock while waiting for a write that would never happen.

Signed-off-by: Arthur Silva Sens <[email protected]>
Signed-off-by: Arthur Silva Sens <[email protected]>
Signed-off-by: Arthur Silva Sens <[email protected]>
@ArthurSens ArthurSens marked this pull request as ready for review February 2, 2025 15:25
@ArthurSens ArthurSens requested a review from a team as a code owner February 2, 2025 15:25
Signed-off-by: Arthur Silva Sens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants